feat(display-controller): round-robin between simultaneous live-priority games#372
Merged
ChuckBuilds merged 1 commit intoJun 15, 2026
Conversation
…ity games _check_live_priority() was stateless first-match-wins: it returned the first plugin in registration order with live content, and the post-dwell hold pinned the carousel to it, so when two games were live at once (e.g. a baseball game and a soccer match) the second never showed until the first ended. Add _collect_live_modes() (all currently-live modes, deduped, in registration order) and give _check_live_priority an 'advance' flag. The main rotation calls it with advance=True, which returns the live mode after the one currently shown -- using current_display_mode as the cursor -- so each dwell advances to the next live game and they take turns. The Vegas coordinator and the vegas-active check keep the default non-advancing peek (advance=False), so they only report whether any game is live without spinning the cursor. should_rotate and _apply_live_priority are unchanged; a single live game still holds as before. Adds regression tests to TestDisplayControllerLivePriority. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughRefactors ChangesRound-robin live-priority selection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 6 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/display_controller.py (1)
1543-1569: ⚡ Quick winConsider adding type hints for clarity.
The method signature and return value would benefit from type annotations to make the contract explicit:
def _check_live_priority(self, advance: bool = False) -> Optional[str]:This documents that
advanceis a boolean flag and the method returns either a mode name string orNone.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/display_controller.py` around lines 1543 - 1569, The _check_live_priority method lacks type hints for its parameters and return value, reducing code clarity. Add type annotations to the method signature: annotate the advance parameter as bool with its default value of False, and add a return type annotation of Optional[str] since the method returns either a mode name string or None. This documents the contract explicitly and improves IDE support and code readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/display_controller.py`:
- Around line 1543-1569: The _check_live_priority method lacks type hints for
its parameters and return value, reducing code clarity. Add type annotations to
the method signature: annotate the advance parameter as bool with its default
value of False, and add a return type annotation of Optional[str] since the
method returns either a mode name string or None. This documents the contract
explicitly and improves IDE support and code readability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7ecf0095-c843-4113-aaf3-5f9ee84c2cf3
📒 Files selected for processing (2)
src/display_controller.pytest/test_display_controller.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
_check_live_priority()returns the first plugin in registration order that has live priority + live content, then stops. The post-dwell hold (should_rotate = False) keeps the carousel on that plugin while it stays live. So when two plugins report live content at the same time — e.g. a baseball game and a soccer match — the board pins to whichever registers first and never shows the other until the first game ends. There is no mechanism to alternate between simultaneously-live games.Fix
Make the live-priority selection round-robin instead of first-match-wins, without changing the hold semantics:
_collect_live_modes()returns all currently-live modes across every plugin (deduped, in registration order). For each plugin with live priority + content it takes the specific modes fromget_live_modes()(only those actually registered), falling back to the scanned*_livemode name. A plugin registered under several mode keys (the sports plugins register one per league) contributes each live mode once._check_live_priority(advance=False):advance=True(the main rotation call site) returns the live mode after the one currently shown, usingcurrent_display_modeas the cursor — so each dwell advances to the next live game and they take turns.advance=False(default; used by the Vegas coordinator and the vegas-active check) is a non-advancing peek that returns the live mode already on screen if it is still live, else the first. These only need to know whether any game is live and must not spin the cursor.should_rotateand_apply_live_priorityare unchanged: a single live game still holds in place, ambient modes stay suppressed while any game is live, and_apply_live_priorityalready switches to whatever mode the checker returns and resumes rotation when it returnsNone. Usingcurrent_display_modeas the cursor keeps selection correct as games start and end — there is no separate index to keep in sync.Testing
New tests in
TestDisplayControllerLivePriority:_collect_live_modesdedupes a multi-mode plugin and skips no-content plugins;*_livemode name whenget_live_modesis unhelpful;None.All existing live-priority tests still pass. Verified end-to-end in the RGBMatrixEmulator with two simultaneous test-mode live games (MLB + MiLB): the controller alternated between them each dwell, with ambient modes suppressed — previously it pinned to the first and the second never showed.
Summary by CodeRabbit
Release Notes
New Features
Tests